Skip to content

Draft: implement home screen#6

Open
ParovozikThomas wants to merge 4 commits intomainfrom
Client_main_page
Open

Draft: implement home screen#6
ParovozikThomas wants to merge 4 commits intomainfrom
Client_main_page

Conversation

@ParovozikThomas
Copy link
Collaborator

No description provided.

@coderabbitai
Copy link

coderabbitai bot commented Mar 10, 2026

📝 Walkthrough

Summary by CodeRabbit

Release Notes

  • New Features
    • Added user authentication with login and registration screens
    • Introduced student and teacher role selection during signup
    • Enabled room/connection joining via invite codes
    • Added connection request management with accept/reject functionality
    • Implemented home screen displaying active connections and pending requests
    • Added invite code generation for teachers

Walkthrough

This pull request introduces a complete authentication and connection management system for the SmartJam Android app. It adds authentication APIs (login, register, token refresh), implements a local token storage layer using DataStore, establishes a Retrofit-based networking infrastructure with request/response models, defines a Room database for connection persistence, and provides three main UI screens (Login, Register, Home) with state-driven ViewModels and navigation orchestration.

Changes

Cohort / File(s) Summary
Build Configuration
mobile/build.gradle.kts, mobile/app/build.gradle.kts
Added KSP plugin and extensive dependencies including Jetpack Compose Navigation, Retrofit, OkHttp, DataStore Preferences, Room, Coroutines, and Compose ViewModel support. Configured buildConfig generation with environment-specific BASE_URL values.
Networking Layer
mobile/app/src/main/java/com/smartjam/app/data/api/AuthApi.kt, mobile/app/src/main/java/com/smartjam/app/data/api/SmartJamApi.kt, mobile/app/src/main/java/com/smartjam/app/data/api/AuthInterceptor.kt, mobile/app/src/main/java/com/smartjam/app/data/api/NetworkModule.kt
Defined Retrofit API interfaces for authentication endpoints (register, login, refresh) and main app endpoints (invite code, join room, connections, submissions). Implemented AuthInterceptor for automatic access token injection and NetworkModule for centralized Retrofit configuration with logging and interceptor setup.
Authentication & Token Management
mobile/app/src/main/java/com/smartjam/app/data/local/TokenStorage.kt, mobile/app/src/main/java/com/smartjam/app/domain/repository/AuthRepository.kt
Added DataStore-based token persistence with expiry tracking and TokenStorage providing Flow-based token access. Implemented AuthRepository wrapping API clients and TokenStorage, handling registration, login, token refresh, logout, and token validity checks with proper error handling.
Data Models
mobile/app/src/main/java/com/smartjam/app/data/model/LoginRequest.kt, mobile/app/src/main/java/com/smartjam/app/data/model/LoginResponse.kt, mobile/app/src/main/java/com/smartjam/app/data/model/LoginState.kt, mobile/app/src/main/java/com/smartjam/app/data/model/RegisterRequest.kt, mobile/app/src/main/java/com/smartjam/app/data/model/RefreshRequest.kt, mobile/app/src/main/java/com/smartjam/app/data/model/ConnectionModels.kt, mobile/app/src/main/java/com/smartjam/app/data/model/TaskModels.kt, mobile/app/src/main/java/com/smartjam/app/data/model/CommentModels.kt
Created request/response DTOs for authentication (RegisterRequest, LoginRequest, LoginResponse, RefreshRequest), UI state (LoginState), connection operations (InviteCodeResponse, JoinRequest, ConnectionDto, RespondConnectionRequest), and assignment/submission workflows (CreateAssignmentRequest, CreateSubmissionRequest, PresignedUrlResponse, SubmissionStatusResponse, SendCommentRequest, CommentResponse).
Database Layer
mobile/app/src/main/java/com/smartjam/app/data/local/SmartJamDatabase.kt, mobile/app/src/main/java/com/smartjam/app/data/local/entity/ConnectionEntity.kt, mobile/app/src/main/java/com/smartjam/app/data/local/dao/ConnectionDao.kt
Defined Room database with ConnectionEntity for persisting connection records. Implemented ConnectionDao with reactive getConnectionsFlow query, bulk insert with conflict replacement, and role-based clearing operations.
Domain Models & Repository
mobile/app/src/main/java/com/smartjam/app/domain/model/UserRole.kt, mobile/app/src/main/java/com/smartjam/app/domain/model/Connection.kt, mobile/app/src/main/java/com/smartjam/app/domain/repository/ConnectionRepository.kt, mobile/app/src/main/java/com/smartjam/app/domain/repository/RoomRepository.kt
Added UserRole enum (STUDENT/TEACHER) and Connection domain model. Implemented ConnectionRepository bridging SmartJamApi and ConnectionDao, exposing operations for syncing connections, generating/validating invite codes, joining rooms, and responding to connection requests. Added RoomRepository for join-by-code operations.
UI Navigation & App Setup
mobile/app/src/main/java/com/smartjam/app/MainActivity.kt, mobile/app/src/main/java/com/smartjam/app/ui/navigation/NavGraph.kt
Refactored MainActivity to initialize core dependencies (TokenStorage, Room database, Retrofit clients, repositories) and render SmartJamNavGraph. Introduced NavGraph with Screen sealed class (Login, Register, Home, Room routes) and SmartJamNavGraph composable wiring destinations and navigation callbacks.
Login Screen
mobile/app/src/main/java/com/smartjam/app/ui/screens/login/LoginScreen.kt, mobile/app/src/main/java/com/smartjam/app/ui/screens/login/LoginViewModel.kt
Implemented LoginScreen composable with animated glass-styled UI components (AppleLiquidBackground, AppleGlassTextField, GoldenStringsButton). Created LoginViewModel managing email/password state, login submission, and event-driven navigation with input validation and error handling.
Register Screen
mobile/app/src/main/java/com/smartjam/app/ui/screens/register/RegisterScreen.kt, mobile/app/src/main/java/com/smartjam/app/ui/screens/register/RegisterViewModel.kt
Built RegisterScreen with role selection UI (GlassRoleSelector) and input fields for username, email, password. Implemented RegisterViewModel with client-side validation (email/password regex, password confirmation matching), role selection state, and registration submission logic.
Home Screen
mobile/app/src/main/java/com/smartjam/app/ui/screens/home/HomeScreen.kt, mobile/app/src/main/java/com/smartjam/app/ui/screens/home/HomeViewModel.kt
Created HomeScreen displaying role-based connection management (teachers see pending requests and invite code generation; students see join interface). Implemented HomeViewModel observing connections per role, syncing network data, handling join/invite/response operations, managing loading/error states, and coordinating navigation events.

Sequence Diagram(s)

sequenceDiagram
    actor User
    participant LoginScreen
    participant LoginViewModel
    participant AuthRepository
    participant AuthApi
    participant TokenStorage
    participant NavGraph
    participant HomeScreen

    User->>LoginScreen: Enter credentials
    LoginScreen->>LoginViewModel: onLoginClicked()
    LoginViewModel->>LoginViewModel: Validate input
    LoginViewModel->>AuthRepository: login(email, password)
    AuthRepository->>AuthApi: login(LoginRequest)
    AuthApi-->>AuthRepository: LoginResponse(tokens, expiry)
    AuthRepository->>TokenStorage: saveToken(...)
    TokenStorage-->>AuthRepository: Success
    AuthRepository-->>LoginViewModel: Result.success()
    LoginViewModel->>LoginViewModel: Emit NavigateToHome
    LoginScreen->>NavGraph: onNavigateToHome()
    NavGraph->>HomeScreen: Navigate & render
    HomeScreen->>HomeScreen: Observe connections
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~75 minutes

Suggested reviewers

  • men229

Poem

🐰✨ Tokens hop from store to screen,
Glass buttons glow with authly sheen,
Connections flow, invites take flight,
NavGraphs weave a smart delight!
One whopping PR, full of might! 🌟

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check ❓ Inconclusive No pull request description was provided by the author, making it impossible to assess relevance to the changeset. Add a description explaining the changes, their purpose, and any related context or dependencies.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The PR title 'Draft: implement home screen' directly addresses the HomeScreen.kt addition and related home screen implementation, which is the main focus of the changeset.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch Client_main_page

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Tip

You can enable review details to help with troubleshooting, context usage and more.

Enable the reviews.review_details setting to include review details such as the model used, the time taken for each step and more in the review comments.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 29

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@mobile/app/build.gradle.kts`:
- Around line 64-65: The inline comment "//ne pon" above the dependency
implementation("androidx.lifecycle:lifecycle-viewmodel-compose:2.8.+") is
unclear—either replace it with a short descriptive comment stating that this
dependency provides Jetpack Lifecycle ViewModel integration for Compose (enables
using ViewModel with Compose lifecycle-aware state) or remove the comment if
redundant; update the comment near the implementation(...) line so readers
understand why the dependency is included.
- Around line 44-46: The dependency version is malformed: the implementation
line uses "$nav_version+" which yields "2.9.7+" (invalid). Fix by removing the
trailing "+" from the implementation usage so it uses the exact nav_version
value, or if you intended a wildcard, change the nav_version declaration to a
segment wildcard like "2.9.+" and keep the implementation as
implementation("androidx.navigation:navigation-compose:$nav_version"); update
the nav_version variable or the implementation string accordingly (refer to the
nav_version variable and the
implementation("androidx.navigation:navigation-compose:$nav_version+")
occurrence).
- Around line 49-65: The build file uses dynamic versions with '+' (e.g.,
dependency coordinates in build.gradle.kts like
"com.squareup.retrofit2:retrofit:2.11.+", "com.squareup.okhttp3:okhttp:4.12.+",
"com.squareup.retrofit2:converter-gson:2.11.+",
"com.squareup.okhttp3:logging-interceptor:4.12.+",
"androidx.datastore:datastore-preferences:1.1.+",
"org.jetbrains.kotlinx:kotlinx-coroutines-android:1.9.+",
"androidx.lifecycle:lifecycle-viewmodel-compose:2.8.+") which makes builds
non-reproducible; replace each '+' wildcard with a specific, exact version
number (pin each artifact to a concrete release) by looking up the current
stable versions for Retrofit, OkHttp, converter-gson, logging-interceptor,
datastore-preferences, kotlinx-coroutines-android, and
lifecycle-viewmodel-compose and update the corresponding implementation(...)
lines accordingly, and optionally centralize those version numbers in a versions
object or Gradle extra properties for maintainability.

In `@mobile/app/src/main/java/com/smartjam/app/data/api/AuthApi.kt`:
- Around line 13-20: The `@POST` annotation for the refresh endpoint uses a
relative path ("api/auth/refresh") while register and login use absolute paths
("/api/auth/..."), which can cause BASE_URL-dependent bugs; update the `@POST` on
the refresh function (refresh) to use the leading slash ("/api/auth/refresh") so
all endpoints (register, login, refresh) use consistent absolute path formatting
in AuthApi.kt.

In `@mobile/app/src/main/java/com/smartjam/app/data/api/NetworkModule.kt`:
- Around line 32-36: The loggingInterceptor currently uses
HttpLoggingInterceptor.Level.BODY which can leak sensitive data; update the lazy
initializer for loggingInterceptor to enable BODY only in debug builds by
checking BuildConfig.DEBUG (import BuildConfig), otherwise use a safer level
(e.g., NONE or BASIC); reference the loggingInterceptor property and the
HttpLoggingInterceptor.Level enum when making the conditional change.
- Around line 14-45: authApi and roomApi each build an identical Retrofit
instance; extract a single shared Retrofit instance (e.g., a private val
retrofit by lazy) configured with BASE_URL, okHttpClient and
GsonConverterFactory, then use retrofit.create(AuthApi::class.java) and
retrofit.create(RoomApi::class.java) to create authApi and roomApi. Keep the
existing okHttpClient and loggingInterceptor as-is and ensure the new shared
Retrofit is referenced by the existing authApi and roomApi properties (use the
same lazy pattern).
- Line 12: The BASE_URL constant in NetworkModule.kt is malformed
("http:/10.0.2.2:8000/") which breaks all network calls; update the private
const val BASE_URL in NetworkModule.kt to the correct URL string
"http://10.0.2.2:8000/" (note the double slash after "http:") so Retrofit/HTTP
client uses a valid base URL.

In `@mobile/app/src/main/java/com/smartjam/app/data/api/RoomApi.kt`:
- Around line 9-12: The joinRoom endpoint in RoomApi lacks Authorization; either
add a `@Header`("Authorization") parameter to suspend fun
joinRoom(`@Header`("Authorization") auth: String, `@Body` request: JoinRoomRequest):
Response<RoomResponse> so callers pass "Bearer <token>", or (preferred) update
your NetworkModule where the OkHttpClient is built to add an interceptor that
reads the stored access token (from your token storage/session manager) and
injects an "Authorization: Bearer <token>" header for every request; modify the
client creation code and ensure Retrofit uses that OkHttpClient so RoomApi
requests include the Bearer token automatically.

In `@mobile/app/src/main/java/com/smartjam/app/data/local/TokenStorage.kt`:
- Around line 75-77: The isAuthenticated() function currently returns false when
the access token is expired, which forces logout instead of allowing a refresh;
change isAuthenticated() to consider the session authenticated as long as the
refresh token is not expired (i.e., return !isRefreshTokenExpired()), so an
expired access token will trigger the refresh flow instead of immediate
unauthentication; update any callers or comments if they rely on the old
behavior and ensure isAccessTokenExpired() is only used to decide whether to
call the refresh routine, not to mark the session unauthenticated.
- Around line 27-33: The tokens are being persisted in plaintext to Preferences
DataStore via saveToken (storing ACCESS_TOKEN, REFRESH_TOKEN, ACCESS_EXPIRED_AT,
REFRESH_EXPIRED_AT); change this to store encrypted values by using AndroidX
Security (MasterKey/AndroidKeyStore) and encrypting the token strings before
writing and decrypting them when reading, or migrate to an encrypted storage
implementation (EncryptedSharedPreferences or an encrypted DataStore wrapper) so
secrets are protected by the platform keystore; update saveToken to perform
encryption of accessToken/refreshToken and update any accessor functions that
read ACCESS_TOKEN/REFRESH_TOKEN to decrypt them, ensuring the encryption key is
created with MasterKey (AndroidKeyStore) and never hard-coded.

In `@mobile/app/src/main/java/com/smartjam/app/data/model/RegisterRequest.kt`:
- Around line 3-8: Replace the raw String role in the RegisterRequest data class
with the existing UserRole enum type (i.e., change RegisterRequest(role: String)
to RegisterRequest(role: UserRole)), import the enum, and update all call sites
that construct RegisterRequest to pass a UserRole instance (or map from string
via UserRole.valueOf(...) where needed); ensure your JSON library is configured
to serialize enums (for Gson no extra change is needed, for Moshi add the proper
adapter) so the enum is converted to/from its string representation during
network (de)serialization.

In
`@mobile/app/src/main/java/com/smartjam/app/domain/repository/AuthRepository.kt`:
- Around line 93-112: The expiry helpers getAccessTokenExpiredIn and
getRefreshTokenExpiredIn subtract System.currentTimeMillis() (milliseconds) from
expiry values provided by tokenStorage.accessExpiredAt / refreshExpiredAt (epoch
seconds), causing ~1000x error; fix by converting the current time to seconds
(e.g. currTimeSec = System.currentTimeMillis() / 1000) before doing the
subtraction so the math uses the same units as tokenStorage, and keep null
checks and return types unchanged.
- Around line 16-30: The suspend methods register, login, and refreshToken
currently catch Exception which also swallows CancellationException; update each
to first catch CancellationException and rethrow it (e.g., catch (e:
CancellationException) { throw e }) before the generic catch (e: Exception)
block so coroutine cancellation remains cooperative and refreshToken won't clear
tokens on cancellation; locate the handlers in register(), login(), and
refreshToken() and insert the rethrowing CancellationException catch above the
existing Exception catch.

In `@mobile/app/src/main/java/com/smartjam/app/MainActivity.kt`:
- Around line 22-36: The app currently always starts SmartJamNavGraph with
startDestination = Screen.Home.route which lets unauthenticated users land on
Home; check TokenStorage.getAccessToken() at startup and compute a
startDestination variable (use Screen.Home.route when token exists, otherwise
Screen.Login.route) and pass that startDestination into SmartJamNavGraph (update
the call site where SmartJamNavGraph is invoked to accept the computed
startDestination and use TokenStorage/ getAccessToken() to decide the initial
route).
- Line 31: Replace the hardcoded Color(0xFF05050A) used in MainActivity (the
color assignment on the composable background) with your theme color (e.g.,
MaterialTheme.colorScheme.background) and define that hex value in your app
theme color scheme (e.g., in your Theme.kt where dark/light ColorScheme or
Background variable is declared) so the background uses the themed color instead
of a literal.

In `@mobile/app/src/main/java/com/smartjam/app/ui/navigation/NavGraph.kt`:
- Around line 78-83: RoomRepository is being instantiated inside the composable
and will be recreated on every recomposition; wrap the repository creation with
remember to keep a single instance per composition (e.g., use remember {
RoomRepository(NetworkModule.roomApi) }) and then pass that remembered instance
into HomeViewModelFactory when calling viewModel (referencing RoomRepository,
NetworkModule.roomApi, HomeViewModelFactory, and the viewModel call in the
composable).
- Around line 3-5: Remove the unused imports to clean up NavGraph.kt: delete the
imports for Box, fillMaxSize and Text (the symbols: Box, fillMaxSize, Text) at
the top of the file so only required Compose imports remain; verify no other
references to these symbols exist in functions like NavGraph or related
composables before committing.
- Around line 37-40: The NavHost currently uses Screen.Home.route as the
unconditional startDestination, allowing unauthenticated users to land on Home;
change this by computing an isAuthenticated boolean in MainActivity (by checking
the token store for valid tokens) and pass the appropriate start destination
into NavHost (e.g., Screen.Home.route if isAuthenticated true, otherwise the
auth/login route). Locate the NavHost usage in NavGraph and update its
startDestination to use that flag, and ensure MainActivity exposes or injects
isAuthenticated into NavGraph instead of hardcoding Screen.Home.route.

In `@mobile/app/src/main/java/com/smartjam/app/ui/screens/home/HomeScreen.kt`:
- Line 10: Remove the unused import CircularProgressIndicator from
HomeScreen.kt; locate the import statement "import
androidx.compose.material3.CircularProgressIndicator" at the top of the file and
delete it so unused imports are cleaned up and lint warnings are resolved.
- Around line 121-133: The Box in HomeScreen.kt currently displays both
state.errorMessage and state.successMessage simultaneously which can overlap;
update the rendering logic inside that Box (the block that checks
state.errorMessage and state.successMessage) to make them mutually exclusive
(e.g., use if (state.errorMessage != null) { ... } else if (state.successMessage
!= null) { ... }) so only one Text is composed at a time, keeping the existing
color and fontSize for each message and prioritizing the error message when both
are present.

In `@mobile/app/src/main/java/com/smartjam/app/ui/screens/home/HomeViewModel.kt`:
- Around line 32-33: The Channel is created with default RENDEZVOUS capacity
which can drop/suspend sends if no collector is active; change the declaration
of eventChannel to use a buffered capacity (e.g., Channel(capacity =
Channel.BUFFERED) or a small fixed capacity) so send() won’t suspend or lose
events, keep the public events flow as eventChannel.receiveAsFlow(), and update
any send sites that rely on non-blocking behavior if you choose a bounded vs
unlimited capacity; references: eventChannel, HomeEvent, events,
receiveAsFlow().
- Around line 35-37: The current onInviteCodeChanged function trims the input on
every keystroke which blocks users from entering spaces; change
onInviteCodeChanged to store the raw input (remove the .trim()) by updating
_state via onInviteCodeChanged(code) to set inviteCodeInput = code and clear
messages as before, and move trimming/normalization to the submission/validation
path (e.g., wherever invite code is validated/submitted — the submitInviteCode
or validateInviteCode function) so that you call code.trim() there before
processing or comparing.

In `@mobile/app/src/main/java/com/smartjam/app/ui/screens/login/LoginScreen.kt`:
- Around line 47-52: The LoginScreen function must not call viewModel() as a
default parameter since LoginViewModel requires AuthRepository; remove the
default parameter and require callers to pass an instantiated LoginViewModel (or
call viewModel(factory = LoginViewModelFactory(...)) themselves). Update the
LoginScreen signature (referencing LoginScreen and LoginViewModel) to take a
non-default viewModel: LoginViewModel and change all call sites including
LoginScreenModernPreview to supply a LoginViewModel instance or use
viewModel(factory = LoginViewModelFactory(...)); ensure use of
LoginViewModelFactory where appropriate so the AuthRepository dependency is
provided.

In
`@mobile/app/src/main/java/com/smartjam/app/ui/screens/login/LoginViewModel.kt`:
- Around line 57-60: In LoginViewModel, the blank-field validation sets
_state.value = _state.value.copy(errorMessage = "Fill in all fields") but
execution continues and immediately enters viewModelScope.launch, which clears
the error and submits empty credentials; fix by returning early after setting
the error (or move the viewModelScope.launch into the else branch) so that when
currentPassword or currentEmail is blank the method exits and does not call
viewModelScope.launch or the submission logic.
- Around line 63-73: authRepository.login(...) returns Result<Unit>, so the
current LoginViewModel code unconditionally sends
eventChannel.send(LoginEvent.NavigateToHome) even when login failed; change the
try block to inspect the Result returned by authRepository.login(currentEmail,
currentPassword) and only send LoginEvent.NavigateToHome when result.isSuccess,
otherwise set _state.value = _state.value.copy(errorMessage =
result.exceptionOrNull()?.message ?: "Login failed") (or use result.fold to
handle success/failure). Keep the existing finally block that sets isLoading =
false and keep using eventChannel for navigation on success.

In
`@mobile/app/src/main/java/com/smartjam/app/ui/screens/register/RegisterScreen.kt`:
- Around line 161-165: The RegisterScreen's registration button
(GoldenStringsButton) is not disabled during loading, allowing duplicate
submissions; update the button in RegisterScreen.kt to prevent clicks while
state.isLoading by either setting its enabled/disabled prop (e.g., enabled =
!state.isLoading) or short-circuiting the onClick to no-op when state.isLoading,
and keep calling viewModel.onRegisterClicked() only when not loading so
duplicate requests are prevented.
- Line 46: The LocalContext.current value is assigned to a variable named
context but never used; remove the unused declaration "val context =
LocalContext.current" from RegisterScreen (and also delete the corresponding
import of androidx.compose.ui.platform.LocalContext if it becomes unused) so
there are no unused variables or imports left in the RegisterScreen.kt code.
- Line 3: Remove the unused import android.widget.Toast from RegisterScreen.kt
to clean up imports; locate the top-level imports in the RegisterScreen
class/file and delete the Toast import line (ensure no other code references
Toast in functions like RegisterScreen or any nested composables before
removing).

In
`@mobile/app/src/main/java/com/smartjam/app/ui/screens/register/RegisterViewModel.kt`:
- Around line 68-109: Prevent duplicate submissions by checking and setting the
loading flag before launching registration: at the start of onRegisterClicked()
read currentState and return immediately if currentState.isLoading is true; then
set isLoading = true (and clear errorMessage) via _state.update {
it.copy(isLoading = true, errorMessage = null) } before calling
viewModelScope.launch so a second tap sees isLoading and is ignored; keep the
existing post-result _state.update { it.copy(isLoading = false) } and error
handling as-is. Use the existing symbols onRegisterClicked, _state, isLoading,
viewModelScope.launch and authRepository.register to locate where to apply this
change.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 136aee47-d8df-440b-9413-db4684e11449

📥 Commits

Reviewing files that changed from the base of the PR and between 32fba48 and 831aa8d.

📒 Files selected for processing (22)
  • mobile/app/build.gradle.kts
  • mobile/app/src/main/java/com/smartjam/app/MainActivity.kt
  • mobile/app/src/main/java/com/smartjam/app/data/api/AuthApi.kt
  • mobile/app/src/main/java/com/smartjam/app/data/api/NetworkModule.kt
  • mobile/app/src/main/java/com/smartjam/app/data/api/RoomApi.kt
  • mobile/app/src/main/java/com/smartjam/app/data/local/TokenStorage.kt
  • mobile/app/src/main/java/com/smartjam/app/data/model/LoginRequest.kt
  • mobile/app/src/main/java/com/smartjam/app/data/model/LoginResponse.kt
  • mobile/app/src/main/java/com/smartjam/app/data/model/LoginState.kt
  • mobile/app/src/main/java/com/smartjam/app/data/model/RefreshRequest.kt
  • mobile/app/src/main/java/com/smartjam/app/data/model/RegisterRequest.kt
  • mobile/app/src/main/java/com/smartjam/app/data/model/RoomModels.kt
  • mobile/app/src/main/java/com/smartjam/app/domain/data/UserRole.kt
  • mobile/app/src/main/java/com/smartjam/app/domain/repository/AuthRepository.kt
  • mobile/app/src/main/java/com/smartjam/app/domain/repository/RoomRepository.kt
  • mobile/app/src/main/java/com/smartjam/app/ui/navigation/NavGraph.kt
  • mobile/app/src/main/java/com/smartjam/app/ui/screens/home/HomeScreen.kt
  • mobile/app/src/main/java/com/smartjam/app/ui/screens/home/HomeViewModel.kt
  • mobile/app/src/main/java/com/smartjam/app/ui/screens/login/LoginScreen.kt
  • mobile/app/src/main/java/com/smartjam/app/ui/screens/login/LoginViewModel.kt
  • mobile/app/src/main/java/com/smartjam/app/ui/screens/register/RegisterScreen.kt
  • mobile/app/src/main/java/com/smartjam/app/ui/screens/register/RegisterViewModel.kt

Comment on lines +44 to +46
val nav_version = "2.9.7"
// Jetpack Compose integration
implementation("androidx.navigation:navigation-compose:$nav_version+")
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Invalid version format: "$nav_version+" produces "2.9.7+".

Appending + to a complete version string (2.9.7+) is not a valid Maven version specifier. The + wildcard should replace a version segment (e.g., 2.9.+), not be appended to an already-specified version.

Either use the exact version or restructure the pattern:

🐛 Proposed fix
-    val nav_version = "2.9.7"
-    // Jetpack Compose integration
-    implementation("androidx.navigation:navigation-compose:$nav_version+")
+    // Jetpack Compose integration
+    implementation("androidx.navigation:navigation-compose:2.9.7")
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
val nav_version = "2.9.7"
// Jetpack Compose integration
implementation("androidx.navigation:navigation-compose:$nav_version+")
// Jetpack Compose integration
implementation("androidx.navigation:navigation-compose:2.9.7")
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@mobile/app/build.gradle.kts` around lines 44 - 46, The dependency version is
malformed: the implementation line uses "$nav_version+" which yields "2.9.7+"
(invalid). Fix by removing the trailing "+" from the implementation usage so it
uses the exact nav_version value, or if you intended a wildcard, change the
nav_version declaration to a segment wildcard like "2.9.+" and keep the
implementation as
implementation("androidx.navigation:navigation-compose:$nav_version"); update
the nav_version variable or the implementation string accordingly (refer to the
nav_version variable and the
implementation("androidx.navigation:navigation-compose:$nav_version+")
occurrence).

Comment on lines +49 to +65
implementation("com.squareup.retrofit2:retrofit:2.11.+")
implementation("com.squareup.okhttp3:okhttp:4.12.+")

//serialization
implementation("com.squareup.retrofit2:converter-gson:2.11.+")

//logging
implementation("com.squareup.okhttp3:logging-interceptor:4.12.+")

//database
implementation("androidx.datastore:datastore-preferences:1.1.+")

//coroutines
implementation("org.jetbrains.kotlinx:kotlinx-coroutines-android:1.9.+")

//ne pon
implementation("androidx.lifecycle:lifecycle-viewmodel-compose:2.8.+")
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

What are the latest stable versions of retrofit, okhttp, datastore-preferences, kotlinx-coroutines-android, and lifecycle-viewmodel-compose as of March 2026?

💡 Result:

As of March 10, 2026, the latest stable versions are:

  • Retrofit (com.squareup.retrofit2:retrofit): 3.0.0 [1]
  • OkHttp (com.squareup.okhttp3:okhttp): 5.3.2 [2]
  • DataStore Preferences (androidx.datastore:datastore-preferences): 1.2.0 [3]
  • kotlinx-coroutines-android (org.jetbrains.kotlinx:kotlinx-coroutines-android): 1.10.2 [4]
  • lifecycle-viewmodel-compose (androidx.lifecycle:lifecycle-viewmodel-compose): 2.10.0 [5]

Sources
[1] (mvnrepository.com)
[2] (square.github.io)
[3] (mvnrepository.com)
[4] (github.com)
[5] (developer.android.com)

Citations:


Avoid dynamic/floating versions (+) for reproducible builds.

Using + wildcards (e.g., 2.11.+, 4.12.+) makes builds non-reproducible and can unexpectedly introduce breaking changes or security vulnerabilities when a new version is published. Pin dependencies to exact versions for stability. Consider also evaluating newer major versions available (e.g., Retrofit 3.0.0, OkHttp 5.3.2) to benefit from latest features and security patches.

🔒 Proposed fix: pin versions to exact releases

Replace each dynamic version with an exact version number. For example:

-    implementation("com.squareup.retrofit2:retrofit:2.11.+")
+    implementation("com.squareup.retrofit2:retrofit:2.11.0")

Consult the latest stable releases on mvnrepository.com to ensure you're not pinning to outdated versions, especially for major version upgrades (Retrofit 3.x, OkHttp 5.x) that may offer significant improvements.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@mobile/app/build.gradle.kts` around lines 49 - 65, The build file uses
dynamic versions with '+' (e.g., dependency coordinates in build.gradle.kts like
"com.squareup.retrofit2:retrofit:2.11.+", "com.squareup.okhttp3:okhttp:4.12.+",
"com.squareup.retrofit2:converter-gson:2.11.+",
"com.squareup.okhttp3:logging-interceptor:4.12.+",
"androidx.datastore:datastore-preferences:1.1.+",
"org.jetbrains.kotlinx:kotlinx-coroutines-android:1.9.+",
"androidx.lifecycle:lifecycle-viewmodel-compose:2.8.+") which makes builds
non-reproducible; replace each '+' wildcard with a specific, exact version
number (pin each artifact to a concrete release) by looking up the current
stable versions for Retrofit, OkHttp, converter-gson, logging-interceptor,
datastore-preferences, kotlinx-coroutines-android, and
lifecycle-viewmodel-compose and update the corresponding implementation(...)
lines accordingly, and optionally centralize those version numbers in a versions
object or Gradle extra properties for maintainability.

Comment on lines +64 to +65
//ne pon
implementation("androidx.lifecycle:lifecycle-viewmodel-compose:2.8.+")
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Clarify or remove unclear comment.

The comment //ne pon is unclear and appears to be shorthand or non-English. Replace it with a descriptive comment explaining what this dependency is for.

✏️ Suggested clarification
-    //ne pon
-    implementation("androidx.lifecycle:lifecycle-viewmodel-compose:2.8.+")
+    // ViewModel integration for Jetpack Compose
+    implementation("androidx.lifecycle:lifecycle-viewmodel-compose:2.8.7")
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
//ne pon
implementation("androidx.lifecycle:lifecycle-viewmodel-compose:2.8.+")
// ViewModel integration for Jetpack Compose
implementation("androidx.lifecycle:lifecycle-viewmodel-compose:2.8.7")
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@mobile/app/build.gradle.kts` around lines 64 - 65, The inline comment "//ne
pon" above the dependency
implementation("androidx.lifecycle:lifecycle-viewmodel-compose:2.8.+") is
unclear—either replace it with a short descriptive comment stating that this
dependency provides Jetpack Lifecycle ViewModel integration for Compose (enables
using ViewModel with Compose lifecycle-aware state) or remove the comment if
redundant; update the comment near the implementation(...) line so readers
understand why the dependency is included.

@@ -0,0 +1,246 @@
package com.smartjam.app.ui.screens.register

import android.widget.Toast
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Unused import: Toast.

Toast is imported but not used in this file.

Proposed fix
-import android.widget.Toast
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
import android.widget.Toast
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@mobile/app/src/main/java/com/smartjam/app/ui/screens/register/RegisterScreen.kt`
at line 3, Remove the unused import android.widget.Toast from RegisterScreen.kt
to clean up imports; locate the top-level imports in the RegisterScreen
class/file and delete the Toast import line (ensure no other code references
Toast in functions like RegisterScreen or any nested composables before
removing).

onNavigateBack: () -> Unit
) {
val state by viewModel.state.collectAsState()
val context = LocalContext.current
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Unused variable: context.

The context variable is declared but never used.

Proposed fix
-    val context = LocalContext.current

Also remove the import if no longer needed:

-import androidx.compose.ui.platform.LocalContext
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
val context = LocalContext.current
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@mobile/app/src/main/java/com/smartjam/app/ui/screens/register/RegisterScreen.kt`
at line 46, The LocalContext.current value is assigned to a variable named
context but never used; remove the unused declaration "val context =
LocalContext.current" from RegisterScreen (and also delete the corresponding
import of androidx.compose.ui.platform.LocalContext if it becomes unused) so
there are no unused variables or imports left in the RegisterScreen.kt code.

Comment on lines +161 to +165
GoldenStringsButton(
text = if (state.isLoading) "Создание..." else "Зарегистрироваться",
onClick = { viewModel.onRegisterClicked() },
modifier = Modifier.fillMaxWidth()
)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Registration button should be disabled while loading to prevent duplicate submissions.

Unlike the join button in HomeScreen (line 118), this button doesn't disable during loading, allowing users to trigger multiple registration requests.

Proposed fix
             GoldenStringsButton(
                 text = if (state.isLoading) "Создание..." else "Зарегистрироваться",
                 onClick = { viewModel.onRegisterClicked() },
-                modifier = Modifier.fillMaxWidth()
+                modifier = Modifier.fillMaxWidth(),
+                enabled = !state.isLoading
             )
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
GoldenStringsButton(
text = if (state.isLoading) "Создание..." else "Зарегистрироваться",
onClick = { viewModel.onRegisterClicked() },
modifier = Modifier.fillMaxWidth()
)
GoldenStringsButton(
text = if (state.isLoading) "Создание..." else "Зарегистрироваться",
onClick = { viewModel.onRegisterClicked() },
modifier = Modifier.fillMaxWidth(),
enabled = !state.isLoading
)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@mobile/app/src/main/java/com/smartjam/app/ui/screens/register/RegisterScreen.kt`
around lines 161 - 165, The RegisterScreen's registration button
(GoldenStringsButton) is not disabled during loading, allowing duplicate
submissions; update the button in RegisterScreen.kt to prevent clicks while
state.isLoading by either setting its enabled/disabled prop (e.g., enabled =
!state.isLoading) or short-circuiting the onClick to no-op when state.isLoading,
and keep calling viewModel.onRegisterClicked() only when not loading so
duplicate requests are prevented.

Comment on lines +68 to +109
fun onRegisterClicked() {
val currentState = _state.value

if (currentState.usernameInput.isBlank()) {
_state.update { it.copy(errorMessage = "Введите имя пользователя") }
return
}

if (!emailRegex.matches(currentState.emailInput)) {
_state.update { it.copy(errorMessage = "Некорректный формат email") }
return
}

if (!passwordRegex.matches(currentState.passwordInput)) {
_state.update { it.copy(errorMessage = "Пароль: мин. 8 символов, латинские буквы и цифры") }
return
}

if (currentState.passwordInput != currentState.repeatPasswordInput) {
_state.update { it.copy(errorMessage = "Пароли не совпадают") }
return
}

viewModelScope.launch {
_state.update { it.copy(isLoading = true, errorMessage = null) }

val result = authRepository.register(
email = currentState.emailInput,
password = currentState.passwordInput,
username = currentState.usernameInput,
role = currentState.selectedRole.name
)

_state.update { it.copy(isLoading = false) }

if (result.isSuccess) {
eventChannel.send(RegisterEvent.NavigateToHome)
} else {
val error = result.exceptionOrNull()?.message ?: "Ошибка регистрации"
_state.update { it.copy(errorMessage = error) }
}
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Prevent concurrent register() submissions.

Line 91 launches every tap; nothing stops a second tap while isLoading is already true. Registration is non-idempotent, so duplicate requests can create conflicting accounts or misleading “already exists” failures.

Minimal fix
 fun onRegisterClicked() {
+        if (_state.value.isLoading) return
         val currentState = _state.value
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
fun onRegisterClicked() {
val currentState = _state.value
if (currentState.usernameInput.isBlank()) {
_state.update { it.copy(errorMessage = "Введите имя пользователя") }
return
}
if (!emailRegex.matches(currentState.emailInput)) {
_state.update { it.copy(errorMessage = "Некорректный формат email") }
return
}
if (!passwordRegex.matches(currentState.passwordInput)) {
_state.update { it.copy(errorMessage = "Пароль: мин. 8 символов, латинские буквы и цифры") }
return
}
if (currentState.passwordInput != currentState.repeatPasswordInput) {
_state.update { it.copy(errorMessage = "Пароли не совпадают") }
return
}
viewModelScope.launch {
_state.update { it.copy(isLoading = true, errorMessage = null) }
val result = authRepository.register(
email = currentState.emailInput,
password = currentState.passwordInput,
username = currentState.usernameInput,
role = currentState.selectedRole.name
)
_state.update { it.copy(isLoading = false) }
if (result.isSuccess) {
eventChannel.send(RegisterEvent.NavigateToHome)
} else {
val error = result.exceptionOrNull()?.message ?: "Ошибка регистрации"
_state.update { it.copy(errorMessage = error) }
}
}
fun onRegisterClicked() {
if (_state.value.isLoading) return
val currentState = _state.value
if (currentState.usernameInput.isBlank()) {
_state.update { it.copy(errorMessage = "Введите имя пользователя") }
return
}
if (!emailRegex.matches(currentState.emailInput)) {
_state.update { it.copy(errorMessage = "Некорректный формат email") }
return
}
if (!passwordRegex.matches(currentState.passwordInput)) {
_state.update { it.copy(errorMessage = "Пароль: мин. 8 символов, латинские буквы и цифры") }
return
}
if (currentState.passwordInput != currentState.repeatPasswordInput) {
_state.update { it.copy(errorMessage = "Пароли не совпадают") }
return
}
viewModelScope.launch {
_state.update { it.copy(isLoading = true, errorMessage = null) }
val result = authRepository.register(
email = currentState.emailInput,
password = currentState.passwordInput,
username = currentState.usernameInput,
role = currentState.selectedRole.name
)
_state.update { it.copy(isLoading = false) }
if (result.isSuccess) {
eventChannel.send(RegisterEvent.NavigateToHome)
} else {
val error = result.exceptionOrNull()?.message ?: "Ошибка регистрации"
_state.update { it.copy(errorMessage = error) }
}
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@mobile/app/src/main/java/com/smartjam/app/ui/screens/register/RegisterViewModel.kt`
around lines 68 - 109, Prevent duplicate submissions by checking and setting the
loading flag before launching registration: at the start of onRegisterClicked()
read currentState and return immediately if currentState.isLoading is true; then
set isLoading = true (and clear errorMessage) via _state.update {
it.copy(isLoading = true, errorMessage = null) } before calling
viewModelScope.launch so a second tap sees isLoading and is ignored; keep the
existing post-result _state.update { it.copy(isLoading = false) } and error
handling as-is. Use the existing symbols onRegisterClicked, _state, isLoading,
viewModelScope.launch and authRepository.register to locate where to apply this
change.

…and interceptor for wrapping requests in JWT
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 15

♻️ Duplicate comments (6)
mobile/app/src/main/java/com/smartjam/app/MainActivity.kt (1)

52-57: ⚠️ Potential issue | 🟠 Major

Choose the start destination from stored auth state.

SmartJamNavGraph still starts at Home, so this Activity can boot an unauthenticated user straight into the protected flow. Decide between login and home before creating the graph, or pass that decision into the graph explicitly.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@mobile/app/src/main/java/com/smartjam/app/MainActivity.kt` around lines 52 -
57, Decide the start destination from stored auth state before instantiating
SmartJamNavGraph: query the auth state (via authRepository or tokenStorage) in
MainActivity (or a suspend/LaunchedEffect) to determine whether user is
authenticated, then either create SmartJamNavGraph with an explicit
startDestination parameter set to "home" or "login", or pass a boolean like
isAuthenticated into SmartJamNavGraph and have the graph use that to set the
initial route; ensure this decision is made prior to constructing
SmartJamNavGraph (where navController, authRepository, connectionRepository,
tokenStorage are currently passed).
mobile/app/build.gradle.kts (1)

51-72: ⚠️ Potential issue | 🟠 Major

Pin these dependencies to exact versions.

Every + here keeps resolution floating, so local, CI, and release builds can pull different artifacts over time. navigation-compose is floating too because $nav_version+ still resolves dynamically.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@mobile/app/build.gradle.kts` around lines 51 - 72, Update the floating
dependency versions to explicit, fixed versions: replace the dynamic ranges
using "+" (e.g., "androidx.navigation:navigation-compose:$nav_version+",
"com.squareup.retrofit2:retrofit:2.11.+", "com.squareup.okhttp3:okhttp:4.12.+",
"com.squareup.retrofit2:converter-gson:2.11.+",
"com.squareup.okhttp3:logging-interceptor:4.12.+",
"androidx.datastore:datastore-preferences:1.1.+",
"org.jetbrains.kotlinx:kotlinx-coroutines-android:1.9.+", and
"androidx.lifecycle:lifecycle-viewmodel-compose:2.8.+") with exact version
strings (set nav_version to a specific patch like "2.9.7" and use exact artifact
versions for retrofit/okhttp/gson/datastore/coroutines/lifecycle) so dependency
resolution is deterministic across environments.
mobile/app/src/main/java/com/smartjam/app/domain/repository/AuthRepository.kt (1)

103-122: ⚠️ Potential issue | 🟠 Major

Use seconds here, not milliseconds.

TokenStorage treats accessExpiredAt and refreshExpiredAt as epoch seconds, but these helpers subtract raw milliseconds. The remaining lifetime will be off by roughly 1000x and usually negative.

Minimal fix
-        val currTime = System.currentTimeMillis()
+        val currTime = System.currentTimeMillis() / 1000
         return accessExpires - currTime
...
-        val currTime = System.currentTimeMillis()
+        val currTime = System.currentTimeMillis() / 1000
         return refreshExpires - currTime
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@mobile/app/src/main/java/com/smartjam/app/domain/repository/AuthRepository.kt`
around lines 103 - 122, The helpers getAccessTokenExpiredIn and
getRefreshTokenExpiredIn subtract System.currentTimeMillis() (ms) from
tokenStorage.accessExpiredAt / refreshExpiredAt which are stored as epoch
seconds, producing values off by ~1000x; fix by converting current time to
seconds (e.g., use System.currentTimeMillis() / 1000 or
TimeUnit.MILLISECONDS.toSeconds(...)) before subtraction so accessExpiredAt -
currTimeSeconds and refreshExpiredAt - currTimeSeconds return correct remaining
seconds, keeping the existing null checks and return types.
mobile/app/src/main/java/com/smartjam/app/data/local/TokenStorage.kt (1)

14-16: ⚠️ Potential issue | 🟠 Major

Encrypt persisted bearer tokens.

saveToken() still writes access and refresh tokens verbatim into PreferencesDataStore. If app data is extracted from the device, those values are immediately reusable session credentials, so this needs keystore-backed encryption before release.

Also applies to: 27-33

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@mobile/app/src/main/java/com/smartjam/app/data/local/TokenStorage.kt` around
lines 14 - 16, Persisted bearer tokens are being written in plaintext via the
Context.dataStore PreferencesDataStore and saveToken(); change this to
keystore-backed encryption by using Jetpack Security (MasterKey) to encrypt
token values before writing and decrypt after reading. Update saveToken(), any
token-read functions (the getters around lines 27-33), and the Context.dataStore
usage to call helper encryptToken()/decryptToken() that use a MasterKey-backed
Cipher (AES-GCM) or EncryptedSharedPreferences/EncryptedFile under the hood, so
only ciphertext is stored in the DataStore and plaintext is only held
transiently in memory.
mobile/app/src/main/java/com/smartjam/app/ui/navigation/NavGraph.kt (1)

37-40: ⚠️ Potential issue | 🟠 Major

Don't hardcode Home as the initial route.

Line 39 still sends logged-out users straight to home_screen, so the auth flow is bypassed on cold start. Derive the start destination from persisted auth state before constructing NavHost.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@mobile/app/src/main/java/com/smartjam/app/ui/navigation/NavGraph.kt` around
lines 37 - 40, NavHost is currently hardcoded to startDestination =
Screen.Home.route which bypasses auth; compute a startDestination beforehand by
reading the persisted auth state (e.g., isLoggedIn/token) and pass that variable
into NavHost instead of Screen.Home.route. Locate the NavHost usage and replace
the literal with a startDestination derived from your auth persistence check (or
a small loading/composed guard that waits for persisted state via
remember/LaunchedEffect), e.g., determine if user is authenticated then choose
Screen.Home.route or Screen.Login.route and use that variable in NavHost to
ensure the auth flow runs on cold start.
mobile/app/src/main/java/com/smartjam/app/ui/screens/home/HomeViewModel.kt (1)

107-114: ⚠️ Potential issue | 🟡 Minor

Trim the invite code on submit.

The earlier fix moved normalization out of onInviteCodeInputChanged(), but Line 108 still reads the raw text and Line 114 sends it unchanged. A pasted code with leading or trailing spaces will be rejected even when the underlying code is valid.

Small fix
-        val code = _state.value.inviteCodeInput
+        val code = _state.value.inviteCodeInput.trim()
         if (code.isBlank()) return
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@mobile/app/src/main/java/com/smartjam/app/ui/screens/home/HomeViewModel.kt`
around lines 107 - 114, In onJoinRoomClicked(), read and use a trimmed invite
code: replace the raw access of _state.value.inviteCodeInput with a trimmed
value (e.g., val code = _state.value.inviteCodeInput.trim()), use that for the
isBlank() check and pass the trimmed code to
connectionRepository.joinByCode(code); ensure any state updates or logging that
rely on the code use the trimmed variable rather than the original input.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@mobile/app/build.gradle.kts`:
- Around line 74-76: The Room compiler dependency version is out of sync: the
build file adds implementation("androidx.room:room-runtime:2.6.1") and
implementation("androidx.room:room-ktx:2.6.1") but uses
ksp("androidx.room:room-compiler:2.5.0"); update the room-compiler coordinate
used in the ksp(...) declaration to match the runtime/ktx version (2.6.1) so the
compiler and runtime are aligned (change the
ksp("androidx.room:room-compiler:2.5.0") entry to use 2.6.1).

In `@mobile/app/src/main/java/com/smartjam/app/data/api/NetworkModule.kt`:
- Around line 14-37: The current shared loggingInterceptor uses
HttpLoggingInterceptor.Level.BODY which will log sensitive JSON payloads for
both AuthApi and SmartJamApi; change the shared logging level to a non-body
level (e.g., HttpLoggingInterceptor.Level.HEADERS or BASIC) so
createRetrofit(tokenStorage) no longer emits request/response bodies, and add a
new, dedicated factory (e.g., createAuthRetrofit or createAuthOkHttpClient) that
uses a separate HttpLoggingInterceptor with Level.BODY only when you explicitly
need it for non-production debug of non-shared clients; locate and update the
loggingInterceptor lazy val and the createRetrofit function to use the safer
shared logger and add the separate auth-specific factory so MainActivity uses
the appropriate client for AuthApi.

In `@mobile/app/src/main/java/com/smartjam/app/data/local/dao/ConnectionDao.kt`:
- Around line 13-20: Queries getConnectionsFlow and clearConnections filter by
myRole and need an index to avoid table scans; add an `@Index` on the myRole
column in the ConnectionEntity (e.g., `@Entity`(..., indices = [Index(value =
["myRole"])]) referencing the myRole property) and update your Room schema
version plus provide a migration to add the index so existing databases are
handled; ensure the indexed column name matches the field/columnName used by
ConnectionEntity.

In `@mobile/app/src/main/java/com/smartjam/app/domain/model/Connection.kt`:
- Around line 4-9: Change the free-form status String on the Connection data
class to a constrained enum: create a ConnectionStatus enum (e.g., PENDING,
ACTIVE, REJECTED, UNKNOWN/default) and change Connection.status: String to
Connection.status: ConnectionStatus; add a mapping function (e.g.,
ConnectionStatus.from(value: String)) that normalizes backend values
(case-insensitive) and returns UNKNOWN for unexpected values, and update all
construction/deserialization sites and any filtering logic that previously
compared raw strings to use the enum values instead.

In
`@mobile/app/src/main/java/com/smartjam/app/domain/repository/AuthRepository.kt`:
- Around line 75-82: The catch block currently clears tokens for any
non-CancellationException, which logs users out on transient failures; update
the error handling in the function containing this catch so that you only call
tokenStorage.clearTokens() when the exception is a confirmed auth rejection
(e.g., an HttpException with 401/403 or a TokenInvalidException), rethrow
CancellationException as before, and for all other exceptions just return false
(or propagate a retryable error) without clearing tokens; reference
tokenStorage.clearTokens(), CancellationException, and the surrounding catch (e:
Exception) block to locate and implement the check.

In
`@mobile/app/src/main/java/com/smartjam/app/domain/repository/ConnectionRepository.kt`:
- Around line 50-51: The two DAO calls dao.clearConnections(role.name) and
dao.insertConnections(allEntities) must be executed inside a single Room
transaction to avoid leaving the cache empty on failure; either add a new DAO
method (e.g., fun refreshConnectionsForRole(roleName: String, entities:
List<...>)) annotated with `@Transaction` that calls clearConnections(...) then
insertConnections(...), and call that from the repository, or use your
RoomDatabase instance's withTransaction { dao.clearConnections(role.name);
dao.insertConnections(allEntities) } at the call site so both operations commit
or roll back together.

In `@mobile/app/src/main/java/com/smartjam/app/ui/navigation/NavGraph.kt`:
- Around line 23-27: The NavGraph is missing a registered composable for the
Room route (Screen.Room / "room_screen") and the navigation call that goes to
"room_screen" does not include the required connectionId parameter, causing
runtime failures; add a composable destination for Screen.Room in NavGraph that
accepts a connectionId navArg (e.g., navArgument("connectionId") { type =
NavType.StringType }) and update the navigation call from Home to navigate to
the Room route with the connectionId included (e.g., build the route with the
connectionId param) so the destination can read it from NavBackStackEntry and
open the correct room.

In `@mobile/app/src/main/java/com/smartjam/app/ui/screens/home/HomeScreen.kt`:
- Around line 52-57: HomeEvent.NavigateToRoom is emitting a connectionId but the
NavGraph and the onNavigateToRoom handler drop it and don't register a room
route; update the navigation to accept and pass the connectionId (e.g., add a
route that includes a connectionId param in NavGraph.kt and change the
onNavigateToRoom callback used in HomeScreen (the handler invoked for
HomeEvent.NavigateToRoom) to navigate to that route with the
event.connectionId), and ensure the destination/composable for the room screen
is registered to read the connectionId param so tapping a connection opens the
correct room.
- Around line 101-106: The PendingRequestCard actions allow duplicate
submissions because the onAccept/onReject callbacks remain enabled during
state.isLoading; update the caller to pass a disabled flag into
PendingRequestCard (e.g., disabled = state.isLoading) and ensure
PendingRequestCard uses that flag to disable both accept and reject buttons
while true; do the same change for the other usage (the block referencing items
at the second occurrence around lines 261-279) so status-changing requests
cannot be double-submitted until the first response completes.
- Around line 241-245: The KeyboardActions onDone currently calls onJoin()
unconditionally and bypasses the button's guard; update the onDone handler in
KeyboardActions to perform the same check used by the Button (enabled =
!isLoading && inputValue.isNotBlank()) before calling onJoin(), e.g., only
invoke onJoin() when !isLoading and inputValue.isNotBlank() (apply the same
trimming/validation logic if the Button uses it) to prevent empty or duplicate
submissions from the keyboard.
- Around line 166-177: The header's clickable debug role toggle (Column with
Modifier.clickable calling onToggleDebugRole()) must be disabled in non-debug
builds; update HomeScreen to gate the clickable behavior behind
BuildConfig.DEBUG so only debug builds allow toggling the role. Concretely, use
BuildConfig.DEBUG to conditionally add the clickable modifier (or choose a
non-clickable Modifier) when rendering the Column that displays "SmartJam" and
the role text (symbols: Column, Modifier.clickable, onToggleDebugRole(), role,
UserRole.TEACHER) so release builds cannot flip the UI role.

In `@mobile/app/src/main/java/com/smartjam/app/ui/screens/home/HomeViewModel.kt`:
- Around line 107-125: The click handlers (e.g., onJoinRoomClicked) currently
start network writes regardless of state; guard against re-entrancy by checking
_state.value.isLoading at the top of each handler and returning immediately if
true, then atomically set isLoading = true before launching the network call
(and clear it in a finally block) so rapid taps won't start duplicate
viewModelScope.launch calls; apply the same pattern to the other write handlers
that call connectionRepository (the methods launching viewModelScope.launch and
calling connectionRepository.joinByCode, other invite creation/response methods,
eventChannel.send, and syncNetworkData) to ensure only one in-flight request at
a time.
- Around line 181-183: The create() method in HomeViewModelFactory should
validate the incoming modelClass instead of unconditionally casting; update
HomeViewModelFactory.create(modelClass: Class<T>) to check if
modelClass.isAssignableFrom(HomeViewModel::class.java) (or the Kotlin
equivalent) and only then return HomeViewModel(connectionRepository,
authRepository) as T, otherwise throw an IllegalArgumentException with a clear
message referencing modelClass; this mirrors the safer pattern used in
RegisterViewModelFactory and prevents later ClassCastException.
- Around line 68-100: The sync coroutine is launched on viewModelScope inside
syncNetworkData(), so it isn’t cancelled when connectionJob is cancelled and can
overwrite state for a new role; make syncNetworkData cancellable by turning it
into a suspending function (suspend fun syncNetworkData()) or by accepting a
CoroutineScope and running its work in the caller’s coroutine, remove the
internal viewModelScope.launch, and call it directly from inside the
connectionJob coroutine in startObservingConnections so
connectionRepository.syncConnections(_state.value.currentRole) and subsequent
_state updates run as children of connectionJob and will be cancelled on role
switch.

In
`@mobile/app/src/main/java/com/smartjam/app/ui/screens/login/LoginViewModel.kt`:
- Around line 66-80: The generic catch in LoginViewModel around
authRepository.login swallows coroutine cancellation; update the handler to
re-throw CancellationException before handling other exceptions by adding a
specific catch for CancellationException that re-throws (preserving cooperative
cancellation), then keep the existing catch for Exception to update
_state.errorMessage and send events; reference the authRepository.login call,
the catch (e: Exception) block, and _state/update logic and replace the single
catch with two catches (CancellationException re-thrown, Exception handled).

---

Duplicate comments:
In `@mobile/app/build.gradle.kts`:
- Around line 51-72: Update the floating dependency versions to explicit, fixed
versions: replace the dynamic ranges using "+" (e.g.,
"androidx.navigation:navigation-compose:$nav_version+",
"com.squareup.retrofit2:retrofit:2.11.+", "com.squareup.okhttp3:okhttp:4.12.+",
"com.squareup.retrofit2:converter-gson:2.11.+",
"com.squareup.okhttp3:logging-interceptor:4.12.+",
"androidx.datastore:datastore-preferences:1.1.+",
"org.jetbrains.kotlinx:kotlinx-coroutines-android:1.9.+", and
"androidx.lifecycle:lifecycle-viewmodel-compose:2.8.+") with exact version
strings (set nav_version to a specific patch like "2.9.7" and use exact artifact
versions for retrofit/okhttp/gson/datastore/coroutines/lifecycle) so dependency
resolution is deterministic across environments.

In `@mobile/app/src/main/java/com/smartjam/app/data/local/TokenStorage.kt`:
- Around line 14-16: Persisted bearer tokens are being written in plaintext via
the Context.dataStore PreferencesDataStore and saveToken(); change this to
keystore-backed encryption by using Jetpack Security (MasterKey) to encrypt
token values before writing and decrypt after reading. Update saveToken(), any
token-read functions (the getters around lines 27-33), and the Context.dataStore
usage to call helper encryptToken()/decryptToken() that use a MasterKey-backed
Cipher (AES-GCM) or EncryptedSharedPreferences/EncryptedFile under the hood, so
only ciphertext is stored in the DataStore and plaintext is only held
transiently in memory.

In
`@mobile/app/src/main/java/com/smartjam/app/domain/repository/AuthRepository.kt`:
- Around line 103-122: The helpers getAccessTokenExpiredIn and
getRefreshTokenExpiredIn subtract System.currentTimeMillis() (ms) from
tokenStorage.accessExpiredAt / refreshExpiredAt which are stored as epoch
seconds, producing values off by ~1000x; fix by converting current time to
seconds (e.g., use System.currentTimeMillis() / 1000 or
TimeUnit.MILLISECONDS.toSeconds(...)) before subtraction so accessExpiredAt -
currTimeSeconds and refreshExpiredAt - currTimeSeconds return correct remaining
seconds, keeping the existing null checks and return types.

In `@mobile/app/src/main/java/com/smartjam/app/MainActivity.kt`:
- Around line 52-57: Decide the start destination from stored auth state before
instantiating SmartJamNavGraph: query the auth state (via authRepository or
tokenStorage) in MainActivity (or a suspend/LaunchedEffect) to determine whether
user is authenticated, then either create SmartJamNavGraph with an explicit
startDestination parameter set to "home" or "login", or pass a boolean like
isAuthenticated into SmartJamNavGraph and have the graph use that to set the
initial route; ensure this decision is made prior to constructing
SmartJamNavGraph (where navController, authRepository, connectionRepository,
tokenStorage are currently passed).

In `@mobile/app/src/main/java/com/smartjam/app/ui/navigation/NavGraph.kt`:
- Around line 37-40: NavHost is currently hardcoded to startDestination =
Screen.Home.route which bypasses auth; compute a startDestination beforehand by
reading the persisted auth state (e.g., isLoggedIn/token) and pass that variable
into NavHost instead of Screen.Home.route. Locate the NavHost usage and replace
the literal with a startDestination derived from your auth persistence check (or
a small loading/composed guard that waits for persisted state via
remember/LaunchedEffect), e.g., determine if user is authenticated then choose
Screen.Home.route or Screen.Login.route and use that variable in NavHost to
ensure the auth flow runs on cold start.

In `@mobile/app/src/main/java/com/smartjam/app/ui/screens/home/HomeViewModel.kt`:
- Around line 107-114: In onJoinRoomClicked(), read and use a trimmed invite
code: replace the raw access of _state.value.inviteCodeInput with a trimmed
value (e.g., val code = _state.value.inviteCodeInput.trim()), use that for the
isBlank() check and pass the trimmed code to
connectionRepository.joinByCode(code); ensure any state updates or logging that
rely on the code use the trimmed variable rather than the original input.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: ad00841b-c03f-4670-8e57-4f6c9834fc52

📥 Commits

Reviewing files that changed from the base of the PR and between 831aa8d and 0572309.

📒 Files selected for processing (29)
  • mobile/app/build.gradle.kts
  • mobile/app/src/main/java/com/smartjam/app/MainActivity.kt
  • mobile/app/src/main/java/com/smartjam/app/data/api/AuthApi.kt
  • mobile/app/src/main/java/com/smartjam/app/data/api/AuthInterceptor.kt
  • mobile/app/src/main/java/com/smartjam/app/data/api/NetworkModule.kt
  • mobile/app/src/main/java/com/smartjam/app/data/api/SmartJamApi.kt
  • mobile/app/src/main/java/com/smartjam/app/data/local/SmartJamDatabase.kt
  • mobile/app/src/main/java/com/smartjam/app/data/local/TokenStorage.kt
  • mobile/app/src/main/java/com/smartjam/app/data/local/dao/ConnectionDao.kt
  • mobile/app/src/main/java/com/smartjam/app/data/local/entity/ConnectionEntity.kt.kt
  • mobile/app/src/main/java/com/smartjam/app/data/model/CommentModels.kt
  • mobile/app/src/main/java/com/smartjam/app/data/model/ConnectionModels.kt
  • mobile/app/src/main/java/com/smartjam/app/data/model/RegisterRequest.kt
  • mobile/app/src/main/java/com/smartjam/app/data/model/TaskModels.kt
  • mobile/app/src/main/java/com/smartjam/app/domain/model/Connection.kt
  • mobile/app/src/main/java/com/smartjam/app/domain/model/UserRole.kt
  • mobile/app/src/main/java/com/smartjam/app/domain/repository/AuthRepository.kt
  • mobile/app/src/main/java/com/smartjam/app/domain/repository/ConnectionRepository.kt
  • mobile/app/src/main/java/com/smartjam/app/domain/repository/RoomRepository.kt
  • mobile/app/src/main/java/com/smartjam/app/ui/navigation/NavGraph.kt
  • mobile/app/src/main/java/com/smartjam/app/ui/screens/home/HomeScreen.kt
  • mobile/app/src/main/java/com/smartjam/app/ui/screens/home/HomeViewModel.kt
  • mobile/app/src/main/java/com/smartjam/app/ui/screens/login/LoginScreen.kt
  • mobile/app/src/main/java/com/smartjam/app/ui/screens/login/LoginViewModel.kt
  • mobile/app/src/main/java/com/smartjam/app/ui/screens/register/RegisterScreen.kt
  • mobile/app/src/main/java/com/smartjam/app/ui/screens/register/RegisterViewModel.kt
  • mobile/app/src/main/java/com/smartjam/app/ui/screens/room/RoomScreen.kt
  • mobile/app/src/main/java/com/smartjam/app/ui/screens/room/RoomViewModel.kt
  • mobile/build.gradle.kts

Comment on lines +74 to +76
implementation("androidx.room:room-runtime:2.6.1")
implementation("androidx.room:room-ktx:2.6.1")
ksp("androidx.room:room-compiler:2.5.0")
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

cat -n mobile/app/build.gradle.kts | sed -n '70,80p'

Repository: Satlykovs/SmartJam

Length of output: 530


🌐 Web query:

Room library version compatibility 2.6.1 2.5.0 compiler runtime

💡 Result:

Android Room versions 2.6.1 and 2.5.0 are both compatible with standard Android compiler (KAPT/annotationProcessor) and runtime setups. Room 2.6.1 (released Nov 29, 2023) introduces Kotlin code generation option via KSP (room.generateKotlin, available from 2.6.0), but supports both KAPT and KSP processors. No explicit incompatibilities between 2.6.1 and 2.5.0 for compiler/runtime. Key compatibility notes: - Both versions work with kapt("androidx.room:room-compiler:VERSION") or ksp("androidx.room:room-compiler:VERSION") for Kotlin projects. - For Room 2.6.1 with KSP, requires compatible KSP version matching Kotlin (e.g., Kotlin 1.9.21 with KSP 1.9.21-1.0.15; mismatches cause NoSuchMethodError as in KSP issue #1633). - Room Gradle Plugin (from 2.6.0+) requires AGP 8.1+ for schema config. - Runtime (room-runtime) has no version-specific restrictions; depends on androidx.sqlite etc., backward compatible. - Upgrade from 2.5.0 to 2.6.1: Potential KSP/Kotlin version alignment needed; some early 2.6.0 KSP issues fixed in 2.6.1. No official compatibility matrix found, but official docs confirm KSP support added in 2.6.0 without breaking prior setups. Use latest stable Kotlin/AGP/KSP for best results (as of 2026: Kotlin 2.x, AGP 8.x+, KSP 2.x).

Citations:


Align Room compiler version with runtime.

Line 76 uses room-compiler:2.5.0 while lines 74-75 use Room 2.6.1. While these versions are compatible, keeping compiler and runtime versions aligned is a best practice to avoid potential code generation issues.

Suggested fix
-    ksp("androidx.room:room-compiler:2.5.0")
+    ksp("androidx.room:room-compiler:2.6.1")
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
implementation("androidx.room:room-runtime:2.6.1")
implementation("androidx.room:room-ktx:2.6.1")
ksp("androidx.room:room-compiler:2.5.0")
implementation("androidx.room:room-runtime:2.6.1")
implementation("androidx.room:room-ktx:2.6.1")
ksp("androidx.room:room-compiler:2.6.1")
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@mobile/app/build.gradle.kts` around lines 74 - 76, The Room compiler
dependency version is out of sync: the build file adds
implementation("androidx.room:room-runtime:2.6.1") and
implementation("androidx.room:room-ktx:2.6.1") but uses
ksp("androidx.room:room-compiler:2.5.0"); update the room-compiler coordinate
used in the ksp(...) declaration to match the runtime/ktx version (2.6.1) so the
compiler and runtime are aligned (change the
ksp("androidx.room:room-compiler:2.5.0") entry to use 2.6.1).

Comment on lines +14 to +37
fun createRetrofit(tokenStorage: TokenStorage): Retrofit {
val authInterceptor = AuthInterceptor(tokenStorage)

val okHttpClient = OkHttpClient.Builder()
.addInterceptor(authInterceptor)
.addInterceptor(loggingInterceptor)
.build()

return Retrofit.Builder()
.baseUrl(BuildConfig.BASE_URL)
.client(okHttpClient)
.addConverterFactory(GsonConverterFactory.create())
.build()
}

private val loggingInterceptor by lazy {
HttpLoggingInterceptor().apply {
level = if (BuildConfig.DEBUG) {
HttpLoggingInterceptor.Level.BODY
} else{
HttpLoggingInterceptor.Level.NONE
}
}
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, let's explore the repository structure
git ls-files | head -20

Repository: Satlykovs/SmartJam

Length of output: 938


🏁 Script executed:

# Find MainActivity to verify AuthApi creation
fd -t f "MainActivity.kt" mobile/

Repository: Satlykovs/SmartJam

Length of output: 119


🏁 Script executed:

# Find AuthApi interface to see what endpoints exist
fd -t f "AuthApi.kt" mobile/

Repository: Satlykovs/SmartJam

Length of output: 123


🏁 Script executed:

# Look for NetworkModule and related setup
fd -t f "NetworkModule.kt" mobile/

Repository: Satlykovs/SmartJam

Length of output: 129


🏁 Script executed:

cat -n mobile/app/src/main/java/com/smartjam/app/MainActivity.kt

Repository: Satlykovs/SmartJam

Length of output: 2628


🏁 Script executed:

cat -n mobile/app/src/main/java/com/smartjam/app/data/api/AuthApi.kt

Repository: Satlykovs/SmartJam

Length of output: 814


🏁 Script executed:

cat -n mobile/app/src/main/java/com/smartjam/app/data/api/NetworkModule.kt

Repository: Satlykovs/SmartJam

Length of output: 1445


Don't use BODY logging on the shared auth client.

MainActivity creates both AuthApi and SmartJamApi from the Retrofit returned here. Debug Level.BODY logging will dump login, register, and refresh request/response bodies—including credentials and refresh tokens—to logcat. Redacting the Authorization header is insufficient because the secrets are also embedded in the JSON payloads.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@mobile/app/src/main/java/com/smartjam/app/data/api/NetworkModule.kt` around
lines 14 - 37, The current shared loggingInterceptor uses
HttpLoggingInterceptor.Level.BODY which will log sensitive JSON payloads for
both AuthApi and SmartJamApi; change the shared logging level to a non-body
level (e.g., HttpLoggingInterceptor.Level.HEADERS or BASIC) so
createRetrofit(tokenStorage) no longer emits request/response bodies, and add a
new, dedicated factory (e.g., createAuthRetrofit or createAuthOkHttpClient) that
uses a separate HttpLoggingInterceptor with Level.BODY only when you explicitly
need it for non-production debug of non-shared clients; locate and update the
loggingInterceptor lazy val and the createRetrofit function to use the safer
shared logger and add the separate auth-specific factory so MainActivity uses
the appropriate client for AuthApi.

Comment on lines +13 to +20
@Query("SELECT * FROM connections WHERE myRole = :role")
fun getConnectionsFlow(role: String): Flow<List<ConnectionEntity>>

@Insert(onConflict = OnConflictStrategy.REPLACE)
suspend fun insertConnections(connections: List<ConnectionEntity>): List<Long>

@Query("DELETE FROM connections WHERE myRole = :role")
suspend fun clearConnections(role: String): Int
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Add an index for role-filtered queries.

Line 13 and Line 19 both filter by myRole; without an index this becomes full-table scan work as data grows.

📌 Proposed schema tweak (in ConnectionEntity)
+import androidx.room.Index
 import androidx.room.Entity
 import androidx.room.PrimaryKey

-@Entity(tableName = "connections")
+@Entity(
+    tableName = "connections",
+    indices = [Index(value = ["myRole"])]
+)
 data class ConnectionEntity(
     `@PrimaryKey` val connectionId: String,
     val peerId: String,
     val peerName: String,
     val status: String,
     val myRole: String
 )
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@mobile/app/src/main/java/com/smartjam/app/data/local/dao/ConnectionDao.kt`
around lines 13 - 20, Queries getConnectionsFlow and clearConnections filter by
myRole and need an index to avoid table scans; add an `@Index` on the myRole
column in the ConnectionEntity (e.g., `@Entity`(..., indices = [Index(value =
["myRole"])]) referencing the myRole property) and update your Room schema
version plus provide a migration to add the index so existing databases are
handled; ensure the indexed column name matches the field/columnName used by
ConnectionEntity.

Comment on lines +4 to +9
data class Connection(
val id: String,
val peerId: String,
val peerName: String,
val status: String
) No newline at end of file
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Replace free-form status with a constrained type.

Using String here makes state filtering brittle; a typo/case mismatch/new backend value can silently drop items from active/pending UI lists.

✅ Proposed fix (domain-level enum)
+enum class ConnectionStatus {
+    PENDING,
+    ACTIVE
+}
+
 data class Connection(
     val id: String,
     val peerId: String,
     val peerName: String,
-    val status: String
+    val status: ConnectionStatus
 )
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
data class Connection(
val id: String,
val peerId: String,
val peerName: String,
val status: String
)
enum class ConnectionStatus {
PENDING,
ACTIVE
}
data class Connection(
val id: String,
val peerId: String,
val peerName: String,
val status: ConnectionStatus
)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@mobile/app/src/main/java/com/smartjam/app/domain/model/Connection.kt` around
lines 4 - 9, Change the free-form status String on the Connection data class to
a constrained enum: create a ConnectionStatus enum (e.g., PENDING, ACTIVE,
REJECTED, UNKNOWN/default) and change Connection.status: String to
Connection.status: ConnectionStatus; add a mapping function (e.g.,
ConnectionStatus.from(value: String)) that normalizes backend values
(case-insensitive) and returns UNKNOWN for unexpected values, and update all
construction/deserialization sites and any filtering logic that previously
compared raw strings to use the enum values instead.

Comment on lines +75 to +82
} catch (e: Exception){
if (e is CancellationException) {
throw e
}
else{
tokenStorage.clearTokens()
return false
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Don't erase the session on every refresh failure.

This branch clears local tokens for any non-cancellation exception. A temporary outage during refresh is not proof that the refresh token is invalid, so users will be logged out even when the session is still recoverable. Reserve clearTokens() for confirmed auth rejection and let transient failures retry.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@mobile/app/src/main/java/com/smartjam/app/domain/repository/AuthRepository.kt`
around lines 75 - 82, The catch block currently clears tokens for any
non-CancellationException, which logs users out on transient failures; update
the error handling in the function containing this catch so that you only call
tokenStorage.clearTokens() when the exception is a confirmed auth rejection
(e.g., an HttpException with 401/403 or a TokenInvalidException), rethrow
CancellationException as before, and for all other exceptions just return false
(or propagate a retryable error) without clearing tokens; reference
tokenStorage.clearTokens(), CancellationException, and the surrounding catch (e:
Exception) block to locate and implement the check.

Comment on lines +241 to +245
keyboardOptions = KeyboardOptions(
keyboardType = KeyboardType.Text,
imeAction = ImeAction.Done
),
keyboardActions = KeyboardActions(onDone = { onJoin() }),
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Keep IME submit behind the same validation as the button.

Line 245 calls onJoin() unconditionally. That bypasses enabled = !isLoading && inputValue.isNotBlank() and allows empty or duplicate submissions from the keyboard.

Match the button guard
-                keyboardActions = KeyboardActions(onDone = { onJoin() }),
+                keyboardActions = KeyboardActions(
+                    onDone = {
+                        if (!isLoading && inputValue.isNotBlank()) {
+                            onJoin()
+                        }
+                    }
+                ),
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
keyboardOptions = KeyboardOptions(
keyboardType = KeyboardType.Text,
imeAction = ImeAction.Done
),
keyboardActions = KeyboardActions(onDone = { onJoin() }),
keyboardOptions = KeyboardOptions(
keyboardType = KeyboardType.Text,
imeAction = ImeAction.Done
),
keyboardActions = KeyboardActions(
onDone = {
if (!isLoading && inputValue.isNotBlank()) {
onJoin()
}
}
),
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@mobile/app/src/main/java/com/smartjam/app/ui/screens/home/HomeScreen.kt`
around lines 241 - 245, The KeyboardActions onDone currently calls onJoin()
unconditionally and bypasses the button's guard; update the onDone handler in
KeyboardActions to perform the same check used by the Button (enabled =
!isLoading && inputValue.isNotBlank()) before calling onJoin(), e.g., only
invoke onJoin() when !isLoading and inputValue.isNotBlank() (apply the same
trimming/validation logic if the Button uses it) to prevent empty or duplicate
submissions from the keyboard.

Comment on lines +68 to +100
private fun startObservingConnections() {
connectionJob?.cancel()

connectionJob = viewModelScope.launch {
val role = _state.value.currentRole

launch {
connectionRepository.getConnectionsFlow(role).collect { connections ->
_state.update { currentState ->
currentState.copy(
activeConnections = connections.filter { it.status == "ACTIVE" },
pendingConnections = connections.filter { it.status == "PENDING" }
)
}
}
}

syncNetworkData()
}
}

fun syncNetworkData() {
viewModelScope.launch {
_state.update { it.copy(isLoading = true, errorMessage = null) }

val result = connectionRepository.syncConnections(_state.value.currentRole)

if (result.isFailure) {
_state.update { it.copy(errorMessage = "Не удалось обновить данные с сервера") }
}

_state.update { it.copy(isLoading = false) }
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

find . -type f -name "HomeViewModel.kt"

Repository: Satlykovs/SmartJam

Length of output: 138


🏁 Script executed:

cat -n ./mobile/app/src/main/java/com/smartjam/app/ui/screens/home/HomeViewModel.kt | head -120

Repository: Satlykovs/SmartJam

Length of output: 4808


🏁 Script executed:

rg "syncNetworkData" ./mobile/app/src/main/java/com/smartjam/app/ui/screens/home/HomeViewModel.kt -A 2 -B 2

Repository: Satlykovs/SmartJam

Length of output: 845


Keep sync work cancellable per role switch.

When syncNetworkData() is called at line 85 within connectionJob, it launches a separate viewModelScope coroutine (line 90) that is not a child of connectionJob. When a role switch triggers connectionJob?.cancel() at line 69, the independent sync coroutine continues running. The stale sync job can later read _state.value.currentRole (line 93) which now reflects the new role, and overwrite isLoading or errorMessage with results from the old role. This causes state corruption across role switches.

Suggested direction
+    private var syncJob: Job? = null
+
     private fun startObservingConnections() {
         connectionJob?.cancel()
+        syncJob?.cancel()
 
         connectionJob = viewModelScope.launch {
             val role = _state.value.currentRole
@@
-            syncNetworkData()
+            syncNetworkData(role)
         }
     }
 
-    fun syncNetworkData() {
-        viewModelScope.launch {
+    fun syncNetworkData(role: UserRole = _state.value.currentRole) {
+        syncJob?.cancel()
+        syncJob = viewModelScope.launch {
             _state.update { it.copy(isLoading = true, errorMessage = null) }
 
-            val result = connectionRepository.syncConnections(_state.value.currentRole)
+            val result = connectionRepository.syncConnections(role)
+            if (_state.value.currentRole != role) return@launch
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@mobile/app/src/main/java/com/smartjam/app/ui/screens/home/HomeViewModel.kt`
around lines 68 - 100, The sync coroutine is launched on viewModelScope inside
syncNetworkData(), so it isn’t cancelled when connectionJob is cancelled and can
overwrite state for a new role; make syncNetworkData cancellable by turning it
into a suspending function (suspend fun syncNetworkData()) or by accepting a
CoroutineScope and running its work in the caller’s coroutine, remove the
internal viewModelScope.launch, and call it directly from inside the
connectionJob coroutine in startObservingConnections so
connectionRepository.syncConnections(_state.value.currentRole) and subsequent
_state updates run as children of connectionJob and will be cancelled on role
switch.

Comment on lines +107 to +125
fun onJoinRoomClicked() {
val code = _state.value.inviteCodeInput
if (code.isBlank()) return

viewModelScope.launch {
_state.update { it.copy(isLoading = true, errorMessage = null) }

val result = connectionRepository.joinByCode(code)

_state.update { it.copy(isLoading = false) }

if (result.isSuccess) {
_state.update { it.copy(inviteCodeInput = "") }
eventChannel.send(HomeEvent.ShowToast("Заявка успешно отправлена!"))
syncNetworkData()
} else {
_state.update { it.copy(errorMessage = "Неверный код или ошибка сервера") }
}
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Block re-entrant write actions while a request is in flight.

Line 111, Line 129, and Line 145 all launch backend writes even if isLoading is already true. Rapid taps can therefore submit duplicate join requests, generate multiple invite codes, or double-handle the same pending request.

Minimal fix
     fun onJoinRoomClicked() {
+        if (_state.value.isLoading) return
         val code = _state.value.inviteCodeInput
         if (code.isBlank()) return
@@
     fun onGenerateCodeClicked() {
+        if (_state.value.isLoading) return
         viewModelScope.launch {
@@
     fun onRespondToRequest(connectionId: String, accept: Boolean) {
+        if (_state.value.isLoading) return
         viewModelScope.launch {

Also applies to: 128-141, 144-159

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@mobile/app/src/main/java/com/smartjam/app/ui/screens/home/HomeViewModel.kt`
around lines 107 - 125, The click handlers (e.g., onJoinRoomClicked) currently
start network writes regardless of state; guard against re-entrancy by checking
_state.value.isLoading at the top of each handler and returning immediately if
true, then atomically set isLoading = true before launching the network call
(and clear it in a finally block) so rapid taps won't start duplicate
viewModelScope.launch calls; apply the same pattern to the other write handlers
that call connectionRepository (the methods launching viewModelScope.launch and
calling connectionRepository.joinByCode, other invite creation/response methods,
eventChannel.send, and syncNetworkData) to ensure only one in-flight request at
a time.

Comment on lines +181 to +183
override fun <T : ViewModel> create(modelClass: Class<T>): T {
return HomeViewModel(connectionRepository, authRepository) as T
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Find HomeViewModel.kt and examine the factory implementation
find . -type f -name "HomeViewModel.kt" | head -20

Repository: Satlykovs/SmartJam

Length of output: 138


🏁 Script executed:

#!/bin/bash
# Find RegisterViewModelFactory to see the safer pattern
find . -type f -name "RegisterViewModelFactory.kt" -o -name "*RegisterViewModelFactory*" | head -20

Repository: Satlykovs/SmartJam

Length of output: 44


🏁 Script executed:

#!/bin/bash
# Read HomeViewModel.kt to see the factory implementation
cat -n ./mobile/app/src/main/java/com/smartjam/app/ui/screens/home/HomeViewModel.kt | sed -n '175,190p'

Repository: Satlykovs/SmartJam

Length of output: 478


🏁 Script executed:

#!/bin/bash
# Search for RegisterViewModelFactory content in the codebase
rg "RegisterViewModelFactory" --type kotlin -A 15 | head -100

Repository: Satlykovs/SmartJam

Length of output: 4564


Add validation for modelClass parameter to HomeViewModelFactory.

The create() method at lines 181-183 returns a HomeViewModel cast to any requested type without validation. If this factory is ever passed a different ViewModel class, the error manifests as a confusing ClassCastException later instead of a clear IllegalArgumentException. The RegisterViewModelFactory in this PR already implements the safer pattern with validation.

Suggested fix
     `@Suppress`("UNCHECKED_CAST")
     override fun <T : ViewModel> create(modelClass: Class<T>): T {
+        if (modelClass.isAssignableFrom(HomeViewModel::class.java)) {
             return HomeViewModel(connectionRepository, authRepository) as T
+        }
+        throw IllegalArgumentException("Unknown ViewModel class")
     }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@mobile/app/src/main/java/com/smartjam/app/ui/screens/home/HomeViewModel.kt`
around lines 181 - 183, The create() method in HomeViewModelFactory should
validate the incoming modelClass instead of unconditionally casting; update
HomeViewModelFactory.create(modelClass: Class<T>) to check if
modelClass.isAssignableFrom(HomeViewModel::class.java) (or the Kotlin
equivalent) and only then return HomeViewModel(connectionRepository,
authRepository) as T, otherwise throw an IllegalArgumentException with a clear
message referencing modelClass; this mirrors the safer pattern used in
RegisterViewModelFactory and prevents later ClassCastException.

Comment on lines +66 to +80
try {
val result = authRepository.login(currentEmail, currentPassword)

if (result.isSuccess){
eventChannel.send(LoginEvent.NavigateToHome)
}
else{
val error = result.exceptionOrNull()?.message ?: "Error"
_state.update { it.copy(errorMessage = error) }
}
} catch (e: Exception){
_state.value = _state.value.copy(
errorMessage = e.message?: "Unknown error"
)
} finally {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

In Kotlin coroutines on Android, does catch (e: Exception)also catchCancellationException, and should CancellationExceptionbe rethrown fromviewModelScope.launch?

💡 Result:

Yes, catch (e: Exception) catches CancellationException because CancellationException is a subclass of Exception. However, if caught, it should be rethrown (or coroutineContext.ensureActive called) to ensure proper coroutine termination and structured concurrency. In viewModelScope.launch, catching CancellationException without rethrowing prevents the coroutine from terminating correctly when the ViewModel is cleared, potentially leading to zombie coroutines or continued execution after cancellation. Official docs and best practices recommend rethrowing it after any necessary cleanup, or using ensureActive in catch blocks with broad exception catching.

Citations:


Re-throw CancellationException before the generic catch.

authRepository.login() rethrows coroutine cancellation, but Line 76's catch (e: Exception) swallows it and converts cancellation into an error state update. This breaks cooperative cancellation semantics—when the ViewModel is cleared, the coroutine won't terminate cleanly.

Fix
+import kotlinx.coroutines.CancellationException
...
-            } catch (e: Exception){
+            } catch (e: CancellationException) {
+                throw e
+            } catch (e: Exception){
                 _state.value = _state.value.copy(
                     errorMessage = e.message?: "Unknown error"
                 )
🧰 Tools
🪛 detekt (1.23.8)

[warning] 76-76: The caught exception is too generic. Prefer catching specific exceptions to the case that is currently handled.

(detekt.exceptions.TooGenericExceptionCaught)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@mobile/app/src/main/java/com/smartjam/app/ui/screens/login/LoginViewModel.kt`
around lines 66 - 80, The generic catch in LoginViewModel around
authRepository.login swallows coroutine cancellation; update the handler to
re-throw CancellationException before handling other exceptions by adding a
specific catch for CancellationException that re-throws (preserving cooperative
cancellation), then keep the existing catch for Exception to update
_state.errorMessage and send events; reference the authRepository.login call,
the catch (e: Exception) block, and _state/update logic and replace the single
catch with two catches (CancellationException re-thrown, Exception handled).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant